-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix break in DirectoryServices SearchResultCollection #113775
base: main
Are you sure you want to change the base?
Conversation
Added enumeration handling when the internal ArrayList has already been loaded but an enumeration follows. This fixes the issue with 9.x Linq statements against the collection caused by the access of the .Count Fixes dotnet#113265
I think the code is the simplest way to fix this issue without making tons more changes. I'm not sure my test class is useful/valid because I'm not on a LDAP domain-joined machine so it simply bypasses for me in local runs. :pointup: @BillArmstrong @stephentoub @tarekgh |
/// <devdoc> | ||
/// Supports a simple type-specific wrapper for the underlying cached list | ||
/// </devdoc> | ||
private sealed class AlreadyReadResultsEnumerator : IEnumerator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I will adjust after the Blues game.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @IDisposable
@stephentoub @steveharter could you please have a quick look? Note, this is a candidate for 9.0 service porting .
Added enumeration handling when the internal ArrayList has already been loaded but an enumeration follows. This fixes the issue with 9.x Linq statements against the collection caused by the access of the .Count
Fixes #113265
I am not entirely sure the best way to test this, as we never had tests for this class until now so I created a new DirectorySearcherTests.cs (conditionally run when LDAP is available) using the setup shown in the bur report.